Repository navigation
fix(acp): keep SDK prompt owner across session continuations - #6354
Conversation
Provider-error retries can start after the failed attempt retires its active SDK scope. Capture and propagate the original run token so terminal agent events settle the owning prompt.
Keep the captured SDK token in session ownership metadata instead of passing it to AgentPromptOptions, which does not expose that internal field.
snowykr
left a comment
snowykr
left a comment
There was a problem hiding this comment.
Verdict
APPROVED
Summary
This change snapshots the failing SDK attempt’s run token before scope retirement and carries it through both direct and scheduled provider-error retry continuations. The token stays within the existing session ownership and scheduling contracts, while ACP’s complete-correlation guard continues to reject foreign terminals. No merge-blocking defects were identified.
Findings / Required Changes
No blocking or actionable findings.
Non-blocking Observations
- A targeted regression test combining provider-error retry after
agent_end/scope retirement with asserting final ACP prompt correlation would cover the remaining test gap. Existing retry/busy-recovery and terminal-correlation tests cover adjacent behavior, but not their combination. This is an optional coverage improvement, not a demonstrated defect.
CI / Verification
For reviewed head 60e1f36eead1e4d545af767816a3a423a84978bf, the completed GitHub checks included Affected path validation, Affected path validation / evidence producer, Virtual integration validation, and gjc-state-gates; the exact-SHA CI summary also showed the affected-path test matrix and coding-agent package/type checks passing. The combined commit status was pending with no status contexts. No tests or builds were run as part of this review.
Axis Coverage
| Axis | Verdict | Coverage |
|---|---|---|
| A1 — Intent / Policy / Contract | APPROVED | Captured retry token propagation follows existing SDK/ACP correlation ownership contracts; no persisted/public contract changed. |
| A2 — Architecture / Correctness / Failure | APPROVED | Checked retry acceptance/scheduling, scope lifecycle, cleanup, duplicate/foreign terminal guards, and failure ordering; no reachable regression found. |
| A3 — Security / Privacy / Trust | APPROVED | Token is captured from internal attempt ownership rather than untrusted caller data; no new authority or trust boundary crossing. |
| A4 — Verification / Tests / CI | APPROVED | Adjacent tests and exact-head successful checks reviewed; missing end-to-end correlation/retry combination is optional coverage only. |
| A5 — Context / Compatibility / Platform | APPROVED | Existing scheduler/acceptance and ACP consumer contracts remain intact; no new persistence, platform, or packaging surface and no material duplicate abstraction. |
Limitations
The combined commit status remained pending without status contexts, although the visible exact-head check runs were successful. Local verification mentioned in the PR description was not independently reproduced.
|
Merged into dev.
— |
probepark
changed the title
|
gjc-acp-feedback: signature |
3 participants
Summary
Keep the causal SDK prompt owner when
AgentSession.#scheduleAgentContinueschedules a continuation without an explicit token. The shared producer now captures#activeSdkRunToken(or the active attempt-scope token) at schedule time and avoids clearing a defined owner during inherited continuations.Field evidence from tank / gjc 0.18.7 (2026-10-05): 61
incomplete_correlationdrops, each followed bywatchdog_expired340s later; 40 followed provider retry only, 7 had no retry (todo reminder path), and ~13 were async-result + retry.Test:
injects a continuation for an interactive todo reminder— GREEN after restore. The required discriminating SDK-host lifecycle assertion could not be reached through the current AgentSession test harness without bypassing the production SDK admission path; the focused reminder test proves continuation injection but does not distinguish owner propagation.Acceptance
agent_endcarries originalcommandId/turnIdvia shared owner capture; todo-reminder producer path covered by continuation regression test.Verification
bun test packages/coding-agent/test/agent-session-todo-reminder.test.ts -t 'injects a continuation for an interactive todo reminder'— passbun test packages/coding-agent/test/agent-session-retry-busy-recovery.test.ts packages/coding-agent/test/agent-session-todo-reminder.test.ts— 13 passbun run --workspaces --if-present check:types— rc 0bun run lint— rc 0#scheduleAgentContinuefix reverted; current test remained green because it does not observe SDK correlation, so this is blocked.